Skip to content

[architect] refactor: delete production-dead pkg/hub spoke alias layer and duplicate test copies - #6327

Merged
clubanderson merged 3 commits into
v5from
arch/refactor-spoke-aliases
Sep 9, 2026
Merged

[architect] refactor: delete production-dead pkg/hub spoke alias layer and duplicate test copies#6327
clubanderson merged 3 commits into
v5from
arch/refactor-spoke-aliases

Conversation

@kubestellar-hive

Copy link
Copy Markdown
Contributor

Refactor

Cluster claimed: src/pkg/hub/spoke_deleted_aliases.go + the 7 pkg/hub test files that referenced its aliases (sso_test.go, heartbeat_activity_test.go deleted; hub_keys_test.go, image_tag_validation_test.go, small_gaps_coverage_test.go, sso_ed25519_coverage_test.go, terminal_key_per_hive_test.go retargeted) and VerifySSOTokenAcrossKeys in hub_pubkey_generations.go. Disjoint from #6292 (config/backends.conf), #6257 (pkg/dashboard/api.go), and #6309 (none of these files in its 218-file diff).

What this changes structurally and why

After the spoke extraction (#6068), pkg/hub/spoke_deleted_aliases.go kept ~21 pass-through aliases. deadcode ./cmd/hive at v5 tip shows 15 are unreachable from any binary — production callers (pkg/dashboard/session.go, api.go, etc.) already import pkg/hub/spoke directly. The dead aliases only kept hub-side test copies compiling, several via lossy JSON marshal/unmarshal round-trips between the duplicated hub/spoke struct types (silently dropping drifted fields instead of surfacing them).

  • Delete 5 zero-reference aliases: CollectClusterHealth, OpenFDCount, FDSoftLimit, RolloutRestartSelf, UpgradeSelfToSHA
  • Delete pkg/hub/sso_test.go + pkg/hub/heartbeat_activity_test.go — byte-identical duplicates of their pkg/hub/spoke twins (12 tests ran twice per CI shard)
  • Retarget remaining alias calls in 5 mixed test files to spoke.*
  • VerifySSOTokenAcrossKeys now calls spoke.VerifySSOToken directly
  • Keep only the 6 live pass-throughs (QuotaExhaustedProcessCount/AgentReason, HashDashboardToken, SelfImageReleaseChannel, SelfDeploymentImage, MintSSOToken) with a comment pointing new code at pkg/hub/spoke

No behavior change.

Verification

  • go build ./... — ok
  • go test ./pkg/hub — ok, coverage 91.3% → 91.4%
  • Pre-existing (unrelated) failure on this host: TestSpokeKeyResolutionAndRotation in pkg/hub/spoke fails identically at clean v5 tip because host env HIVE_SSO_PUBLIC_KEY leaks into it — non-hermetic test, beaded separately.

Fixes #6326


Filed by architect agent (ACMM L5 — hold-gated mode). Hold-gated: human review required.

— hive: agent=architect backend=copilot model=claude-opus-4-6

@kubestellar-hive kubestellar-hive Bot added the hold label Sep 8, 2026
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Sep 8, 2026
@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 8, 2026
@kubestellar-hive kubestellar-hive Bot added architecture Approved by a Hive merger/owner for auto-merge on green CI agent/architect Created or modified by the architect agent hive/hosted-available-oke-11-placeholder-r05x Approved by a Hive merger/owner for auto-merge on green CI labels Sep 8, 2026
@clubanderson

Copy link
Copy Markdown
Member

Hold-gate review: deletion is production-dead (verified), but the PR is red on errors it introduces. Not lifting hold yet.

Verified against origin/v5 = b65e1bb (this PR's merge-base, behind_by: 0, so the CI reds below are fresh, not the #6310/#6316 stale ones). Grep used git grep -n -w over the whole tree; test files inspected separately.

1. Per-symbol evidence (deleted aliases in src/pkg/hub/spoke_deleted_aliases.go)

Every production caller that spells hub.X for these names imports the spoke package under the alias name hub, so the identifier already resolves to spoke.X, not to this file:

  • src/cmd/hive/hubwire.go:14: hub "github.com/hivecommons/hive/pkg/hub/spoke"
  • src/pkg/dashboard/session.go:12: hub "github.com/hivecommons/hive/pkg/hub/spoke"
  • src/pkg/dashboard/terminal_handoff.go:10: hub "github.com/hivecommons/hive/pkg/hub/spoke"
Deleted symbol Non-test callers of the pkg/hub alias Verdict
InferenceBudgetProvider (type alias) none dead
CollectClusterHealth hubwire.go:899 only, via spoke-as-hub import dead
OpenFDCount hubwire.go:758 only, via spoke-as-hub import dead
FDSoftLimit hubwire.go:759 only, via spoke-as-hub import. heartbeat.go:210 is the struct FIELD FDSoftLimit uint64 \json:"fd_soft_limit"`` on the hub-side report type, which this PR does not touch, so the wire tag is unaffected dead
AgentActivityFor hubwire.go:638,1182 only, via spoke-as-hub import dead in prod, LIVE in tests (see 3)
HeartbeatKickInterval none dead in prod, LIVE in tests (see 3)
spokeAgentSummaries (unexported) none outside the alias file dead
QuotaExhaustedAgentCount none dead in prod, LIVE in tests (see 3)
ProviderLimitHeartbeatFields hubwire.go:699,1188 only, via spoke-as-hub import dead in prod, LIVE in tests (see 3)
RolloutRestartSelf hubwire.go:999,1136 only, via spoke-as-hub import; other hits are comments dead
SwitchImageSelf hubwire.go:402 only, via spoke-as-hub import dead
UpgradeSelfToSHA hubwire.go:1128 only, via spoke-as-hub import dead
VerifySSOToken hub_pubkey_generations.go:365 in-package, retargeted by this PR to spoke.VerifySSOToken dead after retarget
TerminalSigningKey dashboard/session.go:251, terminal_handoff.go:61,115, all via spoke-as-hub import dead
MintTerminalAssertion dashboard/session.go:262, via spoke-as-hub import dead
VerifyTerminalAssertion terminal_handoff.go:115, via spoke-as-hub import dead

Non-Go surfaces: no config keys, openapi fields, or dashboard JS reference any of these names. The only non-Go hits are comments in src/proxy/server.js and src/docs/design/master-key-rotation.md saying the JS mirrors hub.TerminalSigningKey / hub.VerifyTerminalAssertion; those describe the spoke implementation and stay accurate. None of the deleted items is a struct field, so no JSON tag is removed.

2. Duplicate tests

diff of pkg/hub/sso_test.go vs pkg/hub/spoke/sso_test.go and of pkg/hub/heartbeat_activity_test.go vs pkg/hub/spoke/heartbeat_activity_test.go: the only differing line in each is package hub vs package spoke. Same 7 + 5 test functions, same assertions. True duplicates.

3. Blocking: the PR breaks the cmd/hive test package

The retarget missed src/cmd/hive/, which imports the real pkg/hub (not the spoke alias) and calls the deleted functions:

  • src/cmd/hive/agent_activity_test.go:47,79,87,93,104,116: hub.AgentActivityFor
  • src/cmd/hive/heartbeat_idle_cadence_test.go:18,23,28: hub.HeartbeatKickInterval
  • src/cmd/hive/config_overrides_replay_test.go:169,190: hub.ProviderLimitHeartbeatFields
  • src/cmd/hive/provider_budget_probe_test.go:247: hub.ProviderLimitHeartbeatFields
  • src/cmd/hive/quota_exhausted_agents_test.go:23,31,34: hub.QuotaExhaustedAgentCount

Check-runs on 6427f1c: golangci-lint, test (rest 1/3), and build-and-test all fail with undefined: hub.AgentActivityFor / hub.ProviderLimitHeartbeatFields / hub.HeartbeatKickInterval (FAIL github.com/hivecommons/hive/cmd/hive [build failed]). The PR body's "go build ./... ok" is true because binaries compile; go vet / go test ./cmd/hive does not.

Also red: changelog-fragment-guard. changelog.d/changed-6326-dead-spoke-aliases.md must start with a - bullet; it currently starts with the emoji.

Needed to lift hold

  1. Retarget the five src/cmd/hive/*_test.go call sites to spoke.* (and switch []hub.AgentSummary literals in those tests to []spoke.AgentSummary where the parameter type changes).
  2. Prefix the changelog fragment with - .

Once that is pushed and lint/test are green I will re-verify and remove hold with lgtm/approved.

@clubanderson

Copy link
Copy Markdown
Member

Review: blocking, CI is red on the head commit (6427f1c) for two reasons, plus coverage did not run.

  1. Build break in cmd/hive tests. The production callers in src/cmd/hive/hubwire.go and src/pkg/dashboard/* import pkg/hub/spoke under the alias hub, so the pass-through aliases are indeed production-dead. But five cmd/hive test files import the real pkg/hub and still call the deleted aliases (deadcode ./cmd/hive does not see test files):
  • src/cmd/hive/agent_activity_test.go -> hub.AgentActivityFor (6 call sites)
  • src/cmd/hive/config_overrides_replay_test.go -> hub.ProviderLimitHeartbeatFields, hub.AgentSummary
  • src/cmd/hive/provider_budget_probe_test.go -> hub.ProviderLimitHeartbeatFields, hub.AgentSummary
  • src/cmd/hive/heartbeat_idle_cadence_test.go -> hub.HeartbeatKickInterval
  • src/cmd/hive/quota_exhausted_agents_test.go -> hub.QuotaExhaustedAgentCount, hub.AgentSummary

golangci-lint, build-and-test (vet) and test (rest 1/3) all fail with undefined: hub.AgentActivityFor / hub.ProviderLimitHeartbeatFields / hub.HeartbeatKickInterval (FAIL github.com/hivecommons/hive/cmd/hive [build failed]). Either retarget those tests to spoke.* (and spoke.AgentSummary) or keep those four aliases.

  1. changelog-fragment-guard fails: changelog.d/changed-6326-dead-spoke-aliases.md must start with a - entry bullet (the compiler owns the headings); the current file starts with an emoji.

Once those are fixed and coverage runs green on v5, this can get lgtm. Note the PR also carries the hold label, which Tide will not merge through.

sec-check and others added 2 commits September 9, 2026 12:09
…plicate hub test copies

pkg/hub/spoke_deleted_aliases.go kept 15 aliases unreachable from any
binary after the spoke extraction (#6068): production callers already
import pkg/hub/spoke directly. Five had zero references anywhere
(CollectClusterHealth, OpenFDCount, FDSoftLimit, RolloutRestartSelf,
UpgradeSelfToSHA); the rest existed only to keep hub-side test copies
compiling, several via lossy JSON round-trips between the duplicated
hub/spoke struct types.

- delete pkg/hub/sso_test.go and pkg/hub/heartbeat_activity_test.go:
  byte-identical duplicates of their pkg/hub/spoke twins
- retarget remaining alias calls in hub_keys_test.go,
  image_tag_validation_test.go, small_gaps_coverage_test.go,
  sso_ed25519_coverage_test.go, terminal_key_per_hive_test.go to spoke.*
- VerifySSOTokenAcrossKeys now calls spoke.VerifySSOToken directly
- spoke_deleted_aliases.go keeps only the six live pass-throughs
  (QuotaExhaustedProcessCount/AgentReason, HashDashboardToken,
  SelfImageReleaseChannel, SelfDeploymentImage, MintSSOToken)

No behavior change. go build ./... ok; go test ./pkg/hub ok.

Refs #6326

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: sec-check <sec-check@hive.kubestellar.io>
The hub alias layer is intentionally removed in PR #6327. Retarget the remaining cmd/hive tests to the canonical pkg/hub/spoke helpers and remove the unused SSO TTL test constant left behind by duplicate test deletion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@clubanderson
clubanderson force-pushed the arch/refactor-spoke-aliases branch from 6427f1c to 7baaf24 Compare September 9, 2026 16:16
@clubanderson

Copy link
Copy Markdown
Member

Fixed the CI breakage after rebasing onto v5.

What changed:

  • Retargeted remaining cmd/hive tests from removed pkg/hub aliases to canonical pkg/hub/spoke helpers.
  • Removed the unused ssoTokenTTL test constant left after deleting duplicate hub tests.
  • No changelog fragment added: test-only/refactor follow-up, no src behavior change.

Verification:

  • go build ./... ✅
  • go vet ./... ✅
  • go test ./cmd/hive/... ./pkg/hub/... ✅
  • golangci-lint run ./cmd/hive/... ./pkg/hub/... ✅
  • go test ./pkg/hub -cover ✅ 89.2% (floor is 87%)

@clubanderson clubanderson added no-changelog No changelog fragment required and removed no-changelog No changelog fragment required labels Sep 9, 2026
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@clubanderson

Copy link
Copy Markdown
Member

/approve

@clubanderson
clubanderson merged commit b1ffce5 into v5 Sep 9, 2026
49 of 50 checks passed
clubanderson added a commit that referenced this pull request Sep 9, 2026
The hub alias layer is intentionally removed in PR #6327. Retarget the remaining cmd/hive tests to the canonical pkg/hub/spoke helpers and remove the unused SSO TTL test constant left behind by duplicate test deletion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@clubanderson
clubanderson deleted the arch/refactor-spoke-aliases branch September 9, 2026 16:50
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clubanderson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent/architect Created or modified by the architect agent approved Indicates a PR has been approved by an approver from all required OWNERS files. architecture Approved by a Hive merger/owner for auto-merge on green CI copilot dco-signoff: yes Indicates the PR's author has signed the DCO. hive/hosted-available-oke-11-placeholder-r05x Approved by a Hive merger/owner for auto-merge on green CI hold no-changelog No changelog fragment required size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant